Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix gcs issue #681 / zenity save dialog not opening on linux #31

Closed
wants to merge 1 commit into from
Closed

Conversation

SIGSEGV111
Copy link

This fixes richardwilkes/gcs#681 .
Tested on openSuSE Tumbleweed with latest updates => zenity version 3.91.0.

@richardwilkes
Copy link
Owner

Looking at the change notes for zenity, no mention of this option being removed is made. It was introduced in version 2.15.1 (2006-03-23) and no other mention has been made of it since. Further looking through the actual commits, I see that commit f17a8bfc875fada1a79e42f6f07119e91a6955f4 on 2021-02-10 by Logan Rathbone unceremoniously removed the option without mentioning it at all (the commit comment: "entry, fileselection: make build against gtk4"). I'm going to make an uneducated guess and assume that the gtk4 system dialog provides the functionality by default, so he decided the option was no longer needed?

Regardless, it is still needed for my purposes. I'd rather not just go without, since it is a very useful feature that helps prevent people from making dumb mistakes. The question is, how? This works perfectly fine with any version prior to the zenity v3.90.0 release, which was created on January 25, 2023. I'd argue they should re-add the option and just ignore it when present (assuming the new code always enables this behavior), just like they've done with other options that they've deprecated in the past, since this is clearly breaking behavior... of course, they've already made the release, so its a bit late for that.

@SIGSEGV111
Copy link
Author

I fully agree.
However there will be a (hopefully) short period were GCS mostly won't work on linux.
You can open and view existing sheets but you cannot modify or create new ones.
Maybe a simple workaround and retry without the option in case of exit code 255?

@richardwilkes
Copy link
Owner

Perhaps. Since you have a system with the new version of Zenity, could you try that approach out and see if it is workable?

@SIGSEGV111
Copy link
Author

I hacked this together and it seems to be working. The screen flickers a bit and then the save dialog appears correctly. The flicker probably comes from the modal dialog handling - creating a window and closing it after the save dialog finishes.

I will update my PR - to give you an idea of the required changes.

jot.Error(errs.Wrap(err))
return
}
if cmd.ProcessState.ExitCode() != 0 {
if cmd.ProcessState.ExitCode() == 255 {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to be redundant

@richardwilkes
Copy link
Owner

I was thinking of something slightly different. Essentially running zenity --help-file-selection and looking for the overwrite option in the output. If it has it, include it in the command line options, if not, omit it, then run as normal. This should avoid any flicker and also not run the risk of the modal being presented twice for inexplicable reasons.

I was going to implement this approach, but then found that -- at least on my distribution -- there is no option for me to update to the broken version of zenity. I looked at the repo, but have no clue how I'd actually build it, not seeing a makefile or other artifacts I'm familiar with.

@richardwilkes
Copy link
Owner

If you can test it, I'm happy to implement the change I suggested and put it up for you to try out.

@SIGSEGV111
Copy link
Author

That is probably way better than what I hacked together with my very limited go knowledge :-)
Just send it over, I will test it.
Thank you!

@richardwilkes
Copy link
Owner

OK, give the code in #32 a try. You can grab it from the fix-zenity branch.

@SIGSEGV111
Copy link
Author

Tested "Save As" for existing characters and the normal "Save" dialog for new characters.
Both work fine now. Thank you!

@richardwilkes
Copy link
Owner

Excellent. I'll close this one out and merge the other. Thanks for testing this.

@SIGSEGV111 SIGSEGV111 deleted the master branch April 12, 2023 17:00
@SIGSEGV111
Copy link
Author

👍
Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Save sheet does not work any more - linuxSaveDialog / zenity broken
2 participants